Add host.docker.internal and host.containers.internal to the dev cert SAN #61265
Conversation
Add new SAN for dev cert + json output for the tool
dotnet-issue-labeler
bot
added
the
area-commandlinetools
|
Looks good so far! |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the certificate generation functionality to include "host.docker.internal" and "host.containers.internal" in the SAN, bumps the certificate version to 3, and adds a new command for JSON output of certificate information.
- Introduces the new --check-json-output option and its logic
- Refactors certificate version handling by separating generated version and minimum acceptable version
- Updates tests and adjusts access modifiers in platform-specific certificate managers
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Tools/dotnet-dev-certs/src/Program.cs | Adds new JSON output option and updates certificate filtering on OSX |
| src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs | Updates tests to use the new MinimumAspNetHttpsCertificateVersion property |
| src/Shared/CertificateGeneration/WindowsCertificateManager.cs, UnixCertificateManager.cs, MacOSCertificateManager.cs | Adjusts access modifiers for the IsExportable method |
| src/Shared/CertificateGeneration/CertificateManager.cs | Bumps certificate version constants, adds new SAN entries, and introduces the MinimumAspNetHttpsCertificateVersion property |
|
Test failure is real. |
| @@ -110,6 +114,10 @@ public static int Main(string[] args) | |||
| "Display warnings and errors only.", | |||
| CommandOptionType.NoValue); | |||
|
|
|||
| var checkJsonOutput = c.Option("--check-trust-machine-readable", | |||
There was a problem hiding this comment.
@DamianEdwards is this good? Last chance to change it 😆
There was a problem hiding this comment.
This is the option to enable the JSON output? Is there not a prior art we can use from the CLI already? @baronfel
There was a problem hiding this comment.
--format json is what we've been doing for most of these. the 'default' format by comparison is text.
There was a problem hiding this comment.
--format jsonis what we've been doing for most of these. the 'default' format by comparison istext.
This option is already used for the certificate export operation. What about --output-format instead? I don't think reusing the same option is a good idea
There was a problem hiding this comment.
output-format is fine with me. Or if we're not going to have any other dev-certs operations support different output types anytime soon, we can just go with --check-trust-machine-readable for now. If that changes in the future and we want more operations to be able to specify output format, we can figure out the consistency story with --format at that point.
There was a problem hiding this comment.
Yep let's not block on this given --format conflicts with the existing option for certificate export format.
|
Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. |
dotnet-policy-service
bot
added
the
pending-ci-rerun
Add host.docker.internal and host.containers.internal to the dev cert SAN
Add necessary SAN for dev container scenario and add an easy to parse output to the dev-certs command.
Description
host.docker.internalandhost.containers.internalto the dev cert SAN.3--check --trustbut with json output:--check-json-outputThe notion of minimum version is important if/when we backport this feature into dotnet 8 and 9: the runtime in 8 or 9 should be able to run perfectly fine without the new SAN in the cert.
Example of output for a v2 certificate (generated with .NET 9):
Example of output for a v3 certificate:
Fixes #61155